Location: php:php-checks/src/main/java/org/sonar/php/checks/phpunit/NoAssertionInTestCheck.java:130

Why is this an issue?

It’s a common pattern to test the result of a java.util.Map.get() against null or calling java.util.Map.containsKey() before proceeding with adding or changing the value in the map. However the java.util.Map API offers a significantly better alternative in the form of the computeIfPresent() and computeIfAbsent() methods. Using these instead leads to cleaner and more readable code.

What changed

This hunk fixes the code smell where Map.containsKey() was used followed by Map.put() with the same key, instead of using the more concise and readable Map.computeIfAbsent() or similar API. The original code checked !assertionInMethod.containsKey(methodDeclaration) and then called assertionInMethod.put(methodDeclaration, false) inside the if-block. The fix replaces this pattern with assertionInMethod.putIfAbsent(methodDeclaration, false) == null, which atomically checks for the key's absence and inserts the value in a single call, making the code cleaner and more idiomatic.

--- a/php-checks/src/main/java/org/sonar/php/checks/phpunit/NoAssertionInTestCheck.java
+++ b/php-checks/src/main/java/org/sonar/php/checks/phpunit/NoAssertionInTestCheck.java
@@ -130,2 +130,1 @@ public class NoAssertionInTestCheck extends PhpUnitCheck {
-      if (!assertionInMethod.containsKey(methodDeclaration)) {
-        assertionInMethod.put(methodDeclaration, false);
+      if (assertionInMethod.putIfAbsent(methodDeclaration, false) == null) {

Location: php:php-checks/src/main/java/org/sonar/php/checks/utils/PhpUnitCheck.java:39

Why is this an issue?

A method that grows too large tends to aggregate too many responsibilities. Such method inevitably become harder to understand and therefore harder to maintain.

What changed

This hunk replaces the beginning of the overly long assertions() method by delegating to four smaller helper methods (arrayContainsAndEqualityAssertions(), fileAndDirectoryAssertions(), typeStateAndIdentityAssertions(), stringXmlJsonAndConstraintAssertions()), each returning a Stream<Assertion>. This splits the original 193-line method into smaller, focused methods, bringing the line count of assertions() well below the 100-line threshold and resolving the code smell about methods that are too large.

--- a/php-checks/src/main/java/org/sonar/php/checks/utils/PhpUnitCheck.java
+++ b/php-checks/src/main/java/org/sonar/php/checks/utils/PhpUnitCheck.java
@@ -39,0 +40,10 @@ public abstract class PhpUnitCheck extends PHPVisitorCheck {
+    return Stream.of(
+      arrayContainsAndEqualityAssertions(),
+      fileAndDirectoryAssertions(),
+      typeStateAndIdentityAssertions(),
+      stringXmlJsonAndConstraintAssertions())
+      .flatMap(s -> s)
+      .collect(Collectors.toMap(e -> e.name().toLowerCase(Locale.ENGLISH), e -> e));
+  }
+
+  private static Stream<Assertion> arrayContainsAndEqualityAssertions() {

Location: php:sonar-php-plugin/src/main/java/org/sonar/plugins/php/reports/phpunit/PhpUnitReportImporter.java:38

Why is this an issue?

Some method calls can effectively be "no-ops", meaning that the invoked method does nothing, based on the application’s configuration (eg: debug logs in production). However, even if the method effectively does nothing, its arguments may still need to evaluated before the method is called.

What changed

This hunk fixes the code smell where methods like reportName() and reportPathKey() are invoked unconditionally as arguments to a logging call. By adding && logger().isInfoEnabled() to the condition, the code ensures that the subsequent logging statement (which calls reportName() and reportPathKey()) is only reached when the logger's info level is actually enabled. This prevents the unnecessary evaluation of those method calls when the log level is too high to display the message, addressing the performance penalty of invoking methods only to pass their results to a no-op logging call.

--- a/sonar-php-plugin/src/main/java/org/sonar/plugins/php/reports/phpunit/PhpUnitReportImporter.java
+++ b/sonar-php-plugin/src/main/java/org/sonar/plugins/php/reports/phpunit/PhpUnitReportImporter.java
@@ -37,1 +37,1 @@ public abstract class PhpUnitReportImporter extends AbstractReportImporter {
-    if (getReportFiles(context).isEmpty()) {
+    if (getReportFiles(context).isEmpty() && logger().isInfoEnabled()) {

Location: php:php-checks/src/main/java/org/sonar/php/checks/DeadStoreCheck.java:94

Why is this an issue?

Cognitive Complexity is a measure of how hard it is to understand the control flow of a unit of code. Code with high cognitive complexity is hard to read, understand, test, and modify.

What changed

This hunk replaces the inline logic for processing symbol usages (which included multiple nested if statements, conditions with logical operators, and loop nesting) with a single method call to processSymbolUsage. By extracting this code into a separate method, the cognitive complexity of the verifyBlock method is reduced because the nested conditionals and their associated complexity increments are moved out of the method. This directly addresses the code smell where the verifyBlock method's cognitive complexity exceeded the allowed threshold of 15.

--- a/php-checks/src/main/java/org/sonar/php/checks/DeadStoreCheck.java
+++ b/php-checks/src/main/java/org/sonar/php/checks/DeadStoreCheck.java
@@ -99,14 +99,1 @@ public class DeadStoreCheck extends PHPSubscriptionCheck {
-        Symbol symbol = symbolWithUsage.getKey();
-        if (outOfScope(readSymbols, symbol)) {
-          // These cases are verified by other checks
-          continue;
-        }
-        VariableUsage usage = symbolWithUsage.getValue();
-        if (usage.isWrite() && !usage.isRead()) {
-          if (!willBeRead.contains(symbol) && !shouldSkip(element, symbol)) {
-            context().newIssue(this, element, String.format(MESSAGE_TEMPLATE, symbol.name()));
-          }
-          willBeRead.remove(symbol);
-        } else if (usage.isRead()) {
-          willBeRead.add(symbol);
-        }
+        processSymbolUsage(symbolWithUsage, readSymbols, willBeRead, element);

Location: php:php-checks/src/main/java/org/sonar/php/checks/phpunit/NoAssertionInTestCheck.java:46

Why is this an issue?

Regular expression engines use backtracking to try all possible execution paths when evaluating a pattern against an input. In some cases, this leads to non-linear backtracking where the worst-case evaluation time grows polynomially (e.g., O(n²) or O(n³)) with the input size. While not as severe as catastrophic backtracking, such patterns can significantly degrade application performance when processing large or untrusted inputs.

What changed

This hunk fixes the regular expression that exhibited super-linear (polynomial) backtracking performance. The original pattern (assert|verify|fail|pass|should|will|check|expect|validate|.*test).* contained .*test inside a group followed by .*, which caused the regex engine to backtrack excessively on non-matching inputs. The fix restructures the pattern to (assert|verify|fail|pass|should|will|check|expect|validate).*|.*test.*, moving the .*test.* alternative outside the group. This eliminates the ambiguous overlap between .*test and the trailing .* quantifier, reducing the backtracking complexity from super-linear to linear.

--- a/php-checks/src/main/java/org/sonar/php/checks/phpunit/NoAssertionInTestCheck.java
+++ b/php-checks/src/main/java/org/sonar/php/checks/phpunit/NoAssertionInTestCheck.java
@@ -46,1 +46,1 @@ public class NoAssertionInTestCheck extends PhpUnitCheck {
-  private static final Pattern ASSERTION_METHODS_PATTERN = Pattern.compile("(assert|verify|fail|pass|should|will|check|expect|validate|.*test).*");
+  private static final Pattern ASSERTION_METHODS_PATTERN = Pattern.compile("(assert|verify|fail|pass|should|will|check|expect|validate).*|.*test.*");